Add a space before "tree" in the "christmas" tree holiday reaction #1404
Conversation
It's reacting to GitHub links that contain /tree/
shenanigansd
changed the title
|
Per discussion on Discord, it looks like we're okay still reacting to most trees, just not when they're in a link. |
|
Are we certain we don't mind most trees? A lot of trees can be stated that have no relation to the season itself. "Grinch's tree", sure, but then what about "skeleton tree"? That will still react with a tree; the regex matches, but that's for halloween. If we're going to activate it for Christmas, I'd rather you match for christmas/xmas then lookahead for tree. |
wookie184
added
the
status: waiting for author
|
Updated to only react specifically to trees of the Christmas variety. |
Xithrius
added
area: backend
| @@ -61,7 +61,7 @@ class Holiday(NamedTuple): | |||
| } | |||
| ) | |||
| Christmas = Holiday([Month.DECEMBER], { | |||
| "christmas tree": Trigger(r"\b((christ|x)mas|tree)\b", ["\U0001F384"]), | |||
| "christmas tree": Trigger(r"\b(christ|x)mas\s?(tree(s)?)?\b", ["\U0001F384"]), | |||
There was a problem hiding this comment.
the parens around the s for pluralizing "tree" are unnecessary, but it might be slightly more readable with them
|
is that a problem? a christmas tree seems to a good reaction for |
That's also the current behavior of the regex. |
|
In that case there's no point mentioning tree in the regex, it can just have the christmas bit |
Relevant Issues
This is one possible solution to #1403. I'll leave the floor open for discussion over other options.Looks like this is the solution that fits!
Closes #1403
Description
christmas|tree->christmas| treeto make sure that we only react to trees of the non-linked variety.regex101 screenshots
Did you: